SDK Consistency Review Summary

I've reviewed this PR for cross-SDK consistency across the TypeScript, Python, Go, and .NET implementations. This is a substantial codegen rewrite that adds RPC method wrappers for all four SDKs.

✅ What's Consistent

The PR does an excellent job maintaining consistency in most areas:

1. Architecture: All SDKs follow the same two-tier pattern:

   • ServerRpc / Client.rpc for server-scoped methods (ping, models, tools, account)
   • SessionRpc / Session.rpc for session-scoped methods (model operations)

2. API Structure: The nested namespace pattern is consistent across all languages:

   • Server level: rpc.ping(), rpc.models.list(), rpc.tools.list(), rpc.account.getQuota()
   • Session level: session.rpc.model.getCurrent(), session.rpc.model.switchTo()

3. Type Generation: All SDKs generate appropriate types from api.schema.json with proper language conventions:

   • TypeScript: camelCase properties, interfaces
   • Python: snake_case properties, dataclasses
   • Go: PascalCase exports, JSON tags
   • .NET: PascalCase, C# attributes

4. Integration: All SDKs expose RPC via lazy-initialized properties on Client and Session classes

⚠️ Consistency Issues Found

Python SDK - Missing Type Hints and Optional Parameters

The Python-generated RPC methods lack type hints and optional parameter defaults that are present in other SDKs. I've left inline comments on:

1. ToolsApi.list(self, model) - should have Optional[str] = None
2. ServerRpc.ping(self, message) - should have Optional[str] = None
3. ModelApi.switch_to(self, model_id) - should have str type hint

These are generated code issues, so the fix should be in the Python codegen script (scripts/codegen/python.ts), not in the generated files themselves.

🔧 Recommendation

Update scripts/codegen/python.ts to:

• Generate type hints for all parameters (e.g., : str, : Optional[str])
• Generate default values for optional parameters (e.g., = None)
• This will bring Python's API signatures into alignment with TypeScript, Go, and .NET

The consistency between the other three SDKs is excellent. Once the Python codegen is updated, all four SDKs will have proper feature parity and consistent API design.

Impact: Low - the Python RPC methods work correctly, they just lack type hints that would improve IDE support and match the conventions of the other SDKs.

AI generated by SDK Consistency Review Agent

All review feedback addressed - see commit details

Cross-SDK Consistency Review

I've reviewed PR #464 for consistency across all four SDK implementations (TypeScript, Python, Go, and .NET). This PR adds RPC code generation and exposes typed RPC methods through client.rpc and session.rpc properties.

✅ Consistent Implementation

The PR does an excellent job maintaining consistency across SDKs:

1. API Surface: All four SDKs expose RPC methods through:

   • Client: client.rpc (TS/Python), client.RPC (Go), client.Rpc (.NET)
   • Session: session.rpc (TS/Python), session.RPC (Go), session.Rpc (.NET)

2. Method Structure: The hierarchical API structure is consistent:

   • Server-scoped: ping(), models.list(), tools.list(), account.getQuota()
   • Session-scoped: model.getCurrent(), model.switchTo()

3. Naming Conventions: Method names follow language idioms correctly:

   • TypeScript: getCurrent()
   • Python: get_current()
   • Go: GetCurrent()
   • .NET: GetCurrentAsync()

4. Integration: All SDKs properly initialize and clean up the RPC objects in start()/Stop() methods

⚠️ Naming Inconsistency Found

There is one naming inconsistency in the C# generated types:

Session model result types use different names:

• ✅ TypeScript: SessionModelGetCurrentResult, SessionModelSwitchToResult
• ✅ Python: SessionModelGetCurrentResult, SessionModelSwitchToResult
• ✅ Go: SessionModelGetCurrentResult, SessionModelSwitchToResult
• ❌ C#: ModelGetCurrentResult, ModelSwitchToResult (missing Session prefix)

Impact: This is a public API type inconsistency. Users working across multiple SDKs would see different type names for the same concept.

Recommendation: Update the C# codegen to include the Session prefix in these type names to match the other three SDKs:

• ModelGetCurrentResult → SessionModelGetCurrentResult
• ModelSwitchToResult → SessionModelSwitchToResult

Summary

The overall implementation is very well done with excellent cross-SDK consistency. The only issue is the missing Session prefix in the C# session model result type names. This should be addressed to maintain consistent type naming across all four SDKs.

AI generated by SDK Consistency Review Agent

Cross-SDK Consistency Review ✅

I've reviewed PR #464 for consistency across all four SDK implementations (Node.js, Python, Go, .NET). Excellent work on maintaining consistency! 🎉

Summary

This PR adds RPC code generation infrastructure to all four SDKs, generating typed wrappers for server and session-scoped RPC methods from a shared api.schema.json schema. The implementation is highly consistent across all languages.

✅ Consistency Verified

1. API Surface
All SDKs expose RPC methods in the same pattern:

• Client-level: client.rpc / client.Rpc / client.RPC for server-scoped methods
• Session-level: session.rpc / session.Rpc / session.RPC for session-scoped methods

2. RPC Methods
All SDKs implement the same complete set of methods:

Server-scoped:

• ping() - Health check with echo
• models.list() - List available models
• tools.list() - List available tools
• account.getQuota() - Get quota information

Session-scoped:

• model.getCurrent() - Get current model for session
• model.switchTo() - Switch session to a different model

3. Naming Conventions
Each SDK correctly follows its language's idiomatic naming:

• Node.js/TypeScript: camelCase (e.g., getQuota, switchTo)
• Python: snake_case (e.g., get_quota, switch_to)
• Go: PascalCase for exported (e.g., GetQuota, SwitchTo)
• .NET: PascalCase with Async suffix (e.g., GetQuotaAsync, SwitchToAsync)

4. Type Definitions
All type definitions serialize to the same JSON wire format while using language-appropriate field names:

// Node.js
interface PingResult { message: string; timestamp: number; protocolVersion: number; }

# Python
`@dataclass`
class PingResult:
    message: str
    protocol_version: float  # snake_case
    timestamp: float

// Go
type PingResult struct {
    Message string `json:"message"`
    ProtocolVersion float64 `json:"protocolVersion"`
    Timestamp float64 `json:"timestamp"`
}

// C#
public class PingResult {
    [JsonPropertyName("message")] public string Message { get; set; }
    [JsonPropertyName("timestamp")] public double Timestamp { get; set; }
    [JsonPropertyName("protocolVersion")] public double ProtocolVersion { get; set; }
}

5. Async Patterns
Each SDK uses its native async pattern:

• Node.js: async/await returning Promise(T)
• Python: async/await returning coroutines
• Go: context.Context for cancellation
• .NET: Task(T) with CancellationToken

6. Error Handling
All SDKs consistently check that the client/session is connected before allowing RPC calls:

• Node.js: Throws Error if not connected
• Python: Lazy initialization on first access
• Go: Field is nil until connected (documented)
• .NET: Throws InvalidOperationException if not started

7. Documentation
All SDKs have consistent documentation:

• Node.js: JSDoc with @throws annotations
• Python: Python docstrings
• Go: Go doc comments
• .NET: XML documentation comments with <summary>, (remarks), (exception)

📋 Optional Enhancement

One minor observation (not blocking): The Node.js SDK doesn't export RPC types from index.ts.

Users who want TypeScript type checking for RPC parameters/results would need to import directly from the generated file:

import type { PingResult, ModelsListResult } from '`@github/copilot-sdk`/generated/rpc';

This might be intentional to keep the public API surface minimal, but if you want users to easily access these types, consider adding to nodejs/src/index.ts:

export type {
    PingResult,
    PingParams,
    ModelsListResult,
    ToolsListResult,
    ToolsListParams,
    AccountGetQuotaResult,
    SessionModelGetCurrentResult,
    SessionModelSwitchToResult,
    SessionModelSwitchToParams,
} from "./generated/rpc.js";

Python, Go, and .NET don't have this concern since:

• Python: Users can import from copilot.generated.rpc
• Go: Exported types in github.com/github/copilot-sdk/go/rpc package
• .NET: Public classes in GitHub.Copilot.SDK.Rpc namespace

However, this is a minor style preference and doesn't impact the core functionality or cross-SDK consistency.

🎯 Conclusion

This PR demonstrates exemplary cross-language consistency. All four SDKs:

• ✅ Implement the same feature set
• ✅ Follow language-specific conventions appropriately
• ✅ Maintain parallel API structures
• ✅ Generate code from a shared schema
• ✅ Include consistent documentation

No blocking consistency issues found. The RPC codegen infrastructure is well-architected and maintains excellent feature parity across all SDK implementations. 🚀

AI generated by SDK Consistency Review Agent

✅ SDK Consistency Review: No Issues Found

I've completed a thorough cross-SDK consistency review of this PR. The RPC codegen implementation demonstrates excellent consistency across all four SDKs (Node.js/TypeScript, Python, Go, and .NET).

Key Findings:

✅ Feature Parity Confirmed:

• All SDKs implement identical server-scoped RPC methods: ping(), models.list(), tools.list(), account.getQuota()
• All SDKs implement identical session-scoped RPC methods: model.getCurrent(), model.switchTo()
• Parameter handling is consistent (optional message in ping, optional model in tools.list, required modelId in model.switchTo)
• Lifecycle management is properly implemented across all SDKs (RPC clients initialized on connect, cleaned up on disconnect)

✅ Appropriate Language Conventions:

• TypeScript: client.rpc, camelCase methods
• Python: client.rpc, snake_case methods
• Go: client.RPC, PascalCase methods (follows Go initialism conventions)
• .NET: client.Rpc, PascalCase + Async suffix

✅ Comprehensive Testing:

• All SDKs include E2E tests for RPC methods with equivalent coverage

✅ Previous Feedback Addressed:
The PR has successfully addressed all consistency issues raised in earlier reviews.

No action required — this PR maintains strong cross-SDK consistency and is ready from an SDK consistency perspective.

AI generated by SDK Consistency Review Agent for #464

AI generated by SDK Consistency Review Agent

Cross-SDK Consistency Review

I've reviewed this PR for consistency across all four SDK implementations. Overall, the RPC codegen work is very well done and maintains excellent feature parity! 🎉

✅ What's Consistent

1. RPC Methods: All 4 SDKs expose identical RPC methods (ping, models.list, tools.list, account.getQuota, session.model.getCurrent, session.model.switchTo)

2. Integration Pattern: All SDKs consistently expose RPC as properties:

   • Node.js/Python: client.rpc / session.rpc
   • Go: client.RPC / session.RPC
   • .NET: client.Rpc / session.Rpc

3. Type Definitions: All generated types have matching fields across languages

4. Test Coverage: All SDKs test the same methods with equivalent coverage

⚠️ API Design Difference: Parameter Passing

I noticed one design difference in how parameters are exposed to developers:

Node.js/Python/Go: Use params objects/structs

// Node.js
client.rpc.ping({ message: "hello" })
client.rpc.tools.list({ model: "claude-sonnet-4.5" })

# Python
client.rpc.ping(PingParams(message="hello"))
client.rpc.tools.list(ToolsListParams(model="claude-sonnet-4.5"))

// Go
client.RPC.Ping(ctx, &rpc.PingParams{Message: &msg})
client.RPC.Tools.List(ctx, &rpc.ToolsListParams{Model: &modelId})

.NET: Unpacks to individual method parameters

// .NET
client.Rpc.PingAsync(message: "hello")
client.Rpc.Tools.ListAsync(model: "claude-sonnet-4.5")

Discussion

This difference is actually language-idiomatic:

• ✅ C# developers typically expect method parameters for 1-2 simple fields
• ✅ TypeScript/Python/Go developers are used to object/struct parameters
• ✅ All SDKs work correctly at the wire protocol level

However, it does create:

• Different developer experience across languages
• Code examples that don't translate 1:1 between languages
• Potential future breaking changes in .NET if more parameters are added

Options

1. Accept as-is (recommended): Document the difference and acknowledge that each SDK follows its language's conventions. This feels most natural to developers in each ecosystem.

2. Standardize on params objects: Change .NET to use params objects everywhere for consistency, though this may feel less natural in C#.

3. Hybrid with overloads: Add both patterns in .NET (individual parameters + params object overloads).

Would love to hear the team's thoughts on this! Given that the wire protocol is identical and each approach is idiomatic for its language, Option 1 seems reasonable to me.

Overall assessment: This PR achieves excellent cross-SDK consistency where it matters most (features, types, behavior). The parameter passing difference is a deliberate design choice that favors language idioms over strict uniformity. ✨

AI generated by SDK Consistency Review Agent

Cross-SDK Consistency Review

I've reviewed this PR for consistency across all four SDK implementations (Node.js/TypeScript, Python, Go, and .NET). Overall, the implementation maintains excellent consistency with appropriate language-specific conventions.

✅ Consistent Implementation

The PR successfully adds typed RPC functionality to all four SDKs with:

1. Client-level RPC access: All SDKs expose server-scoped RPC methods via client.rpc / client.RPC / Client.Rpc
2. Session-level RPC access: All SDKs expose session-scoped RPC methods via session.rpc / session.RPC / session.Rpc
3. Proper naming conventions: Method names follow language idioms:
   • TypeScript: client.rpc.ping(), session.rpc.model.getCurrent()
   • Python: client.rpc.ping(), session.rpc.model.get_current()
   • Go: client.RPC.Ping(), session.RPC.Model.GetCurrent()
   • .NET: Client.Rpc.PingAsync(), session.Rpc.Model.GetCurrentAsync()
4. Cleanup on disconnect: All SDKs properly reset RPC instances in stop() / forceStop() methods
5. E2E test coverage: All languages have equivalent E2E tests with the same test scenarios

⚠️ Identified Inconsistency

Error handling when accessing RPC before client start:

Three SDKs provide helpful error messages when accessing RPC before the client is connected:

• Node.js: Throws Error("Client is not connected. Call start() first.")
• Python: Raises RuntimeError("Client is not connected. Call start() first.")
• .NET: Throws InvalidOperationException("Client is not started. Call StartAsync first.")

However, Go exposes RPC as a public field (*rpc.ServerRpc) that is nil until Start() is called. This means:

client := copilot.NewClient(opts)
// No error - RPC is just nil
result, err := client.RPC.Ping(ctx) // ❌ nil pointer dereference

Suggestion: Consider adding a check in Go's RPC method implementations or documenting this behavior more explicitly in the Go client's godoc for the RPC field.

Minor Note

The error messages have a slight inconsistency:

• Node.js/Python: "Call start() first"
• .NET: "Call StartAsync first"

This is acceptable since it correctly reflects each language's actual method name.

Verdict: This is an excellent multi-language feature implementation. The identified Go error handling difference is a minor issue that could be addressed in a follow-up if desired. The core functionality and API design are consistently implemented across all four SDKs. ✅

AI generated by SDK Consistency Review Agent